Skip to content

Conversation

@zshuang0316
Copy link
Contributor

@zshuang0316 zshuang0316 commented Nov 15, 2025

Set context glob path size correctly.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • [ N/A] Example configuration file for the change
  • Debug log output from testing the change
    [2025/11/15 17:48:33] [debug] [input:blob:blob.0] scanning path C:\data\Logs\local*.log
    [2025/11/15 17:48:33] [ info] [input:blob:blob.0] DEBUG: recursive_file_search called with path='NULL' pattern='C:\data\Logs\local*.log'
    [2025/11/15 17:48:33] [ info] [input:blob:blob.0] DEBUG: calling glob with local_pattern='C:\data\Logs\local*.log'
    [2025/11/15 17:48:33] [ info] [input:blob:blob.0] DEBUG: glob returned 0, found 14 matches
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A ] Run local packaging test showing all targets (including any new ones) build.
  • [N/A ] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [ N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected the count of results returned by glob operations on Windows.
  • Style

    • Fixed file formatting with proper line endings.

Set context glob path size correctly.

Signed-off-by: zshuang0316 <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Nov 15, 2025

Walkthrough

The glob function in the Windows glob implementation is updated to properly set the gl_pathc field with the count of matched entries after populating gl_pathv. A trailing newline is added after the final #endif for formatting consistency.

Changes

Cohort / File(s) Summary
Windows glob counter initialization
plugins/in_blob/win32_glob.c
Set context->gl_pathc = entries to expose the count of matched path results after populating the path vector

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • The change is minimal and localized to a single file
  • Setting a counter field is straightforward
  • Verify that gl_pathc is the correct field for storing the entry count and that entries reflects the accurate matched count

Suggested labels

backport to v4.0.x

Suggested reviewers

  • koleini

Poem

🐰 Glob patterns now count their findings true,
Each path discovered, tallied through and through,
A trailing newline, neat and clean,
The smallest changes, most pristine!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the glob function for win32 in the in_blob plugin, which aligns with setting context->gl_pathc correctly after populating gl_pathv.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
plugins/in_blob/win32_glob.c (1)

107-117: Consider defensive cleanup in globfree.

For consistency and defensive programming, consider:

  1. Setting context->gl_pathc = 0 after freeing gl_pathv
  2. Removing the unused index variable declared on line 109

These changes would make the cleanup more complete and remove dead code.

 static void globfree(glob_t *context)
 {
-    size_t index;
-
     if (context->gl_pathv != NULL) {
         flb_free(context->gl_pathv);
         context->gl_pathv = NULL;
+        context->gl_pathc = 0;
     }

     flb_file_glob_clean(&context->inner_context);
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1ba23a and b1381a1.

📒 Files selected for processing (1)
  • plugins/in_blob/win32_glob.c (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
  • GitHub Check: PR - fuzzing test
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
  • GitHub Check: pr-compile-without-cxx (3.31.6)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-centos-7
🔇 Additional comments (2)
plugins/in_blob/win32_glob.c (2)

153-153: Essential fix: properly exposes the match count.

This correctly sets gl_pathc to the number of matched paths after populating gl_pathv. Without this assignment, callers would have no way to determine how many entries are in the path array, breaking the glob functionality on Windows.


608-608: Good practice: file ends with newline.

This formatting improvement aligns with standard coding conventions.

Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

@zshuang0316
Copy link
Contributor Author

@coderabbitai help could you guide me how to re-run the failed checks? I think they should not relate to my change.

@coderabbitai
Copy link

coderabbitai bot commented Nov 28, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the status of checks/CI runs on this PR
gh pr view 11165 --repo fluent/fluent-bit --json statusCheckRollup,commits | jq '.'

Length of output: 22891


🏁 Script executed:

#!/bin/bash
# Get more details about the PR including check runs
gh api repos/fluent/fluent-bit/pulls/11165/commits | jq '.[-1].sha' -r | xargs -I {} gh api repos/fluent/fluent-bit/commits/{}/check-runs | jq '.check_runs[] | {name: .name, status: .status, conclusion: .conclusion, html_url: .html_url}'

Length of output: 6507


🏁 Script executed:

#!/bin/bash
# Check if there are any workflow runs for this PR
gh run list --repo fluent/fluent-bit --branch patch-4 --limit 10 --json databaseId,status,conclusion,name,event,createdAt,workflowName,url

Length of output: 2445


@zshuang0316, you're correct that the failed check is unrelated to your Windows-specific changes. Let me explain what happened and how to re-run the checks.

What Failed

The only failing check is:

This is a Linux unit test with the undefined behavior sanitizer enabled. Your changes are in plugins/in_blob/win32_glob.c, which is Windows-specific code, so this failure is definitely unrelated to your PR.

Good News

All Windows-related checks passed successfully:

  • ✅ Windows 32bit build and tests
  • ✅ Windows 64bit build and tests
  • ✅ Windows 64bit (Arm64) build and tests

How to Re-run Failed Checks

You have a few options:

Option 1: Ask a Maintainer

Since you've already tagged maintainers as reviewers (@edsiper, @cosmo0920), you can ask them to re-run the failed checks. Maintainers can click the "Re-run failed jobs" button in the Actions tab.

Option 2: Push an Empty Commit (if you have write access)

git commit --allow-empty -m "chore: trigger CI re-run"
git push

Option 3: Wait for Maintainer Review

Often maintainers will recognize flaky tests and either re-run them or merge the PR knowing the failure is unrelated.

Recommendation

I suggest commenting on your PR mentioning the maintainers and noting that the failing test is a Linux sanitizer test unrelated to your Windows-specific fix, and all Windows checks passed. They can then re-run the failed job or proceed with the merge.

Would you like me to help you draft a comment for the maintainers?

@zshuang0316
Copy link
Contributor Author

Hello @cosmo0920, could you please help on the failed linux check which should not related to the code change? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants